fix: RoleCache legacy compat layer ignores platform-wide glob role assignments - #38984
fix: RoleCache legacy compat layer ignores platform-wide glob role assignments#38984efortish wants to merge 1 commit into
Conversation
…signments authz_get_all_course_assignments_for_user() only fetched CourseOverviewData and OrgCourseOverviewGlobData scopes, never PlatformCourseOverviewGlobData (course-v1:*). Even if it had, _get_org_and_course_id_from_authz_scope() had no branch for it, since a platform-wide scope doesn't map to a single org the way course/org-wide scopes do. These assignments feed RoleCache/BulkRoleCache, which back has_access()/ CourseRole.has_user() legacy checks. A user whose only role assignment was a platform-wide glob got an empty RoleCache for every course, silently denying access despite the AuthZ assignment existing. Fix: a platform-wide grant applies to every org, so represent it as an org-wide grant (org, course_id=None) repeated for every registered org (reusing organizations.api.get_organizations(), the same helper used for the analogous fix in openedx-authz#380). This is exactly the shape an org-wide grant already produces, so it's picked up by the existing OrgRole-based legacy checks (has_staff_roles, get_user_permissions, which already check org-level and course-level access separately) with no changes needed to RoleCache/OrgRole/CourseRole. Fixes openedx/openedx-authz#379
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @efortish! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
2 participants
Description
Fixes openedx-authz#379
authz_get_all_course_assignments_for_user()(common/djangoapps/student/roles.py) only fetchedCourseOverviewDataandOrgCourseOverviewGlobDatascope types, but neverPlatformCourseOverviewGlobData(thecourse-v1:*platform-wide glob).Even if it had,
_get_org_and_course_id_from_authz_scope()had no branch for it, since a platform-wide scope does not map to a single org like course- or org-wide scopes do — it applies to every org.These assignments feed
RoleCache/BulkRoleCache, which back legacyhas_access()/CourseRole.has_user()checks throughout the codebase. As a result, a user whose only role assignment was a platform-wide glob (e.g. an instructor oncourse-v1:*) got an emptyRoleCachefor every course, silently denying access even though the AuthZ assignment existed.AuthZ-native checks that consult
IS_PLATFORM_GLOBdirectly (e.g.user_can_create_library) were unaffected.Approach
A platform-wide grant applies to every org, so it is represented as an org-wide grant (
org,course_id=None) repeated for every registered org viaorganizations.api.get_organizations(), using the same approach as the analogous fix in [[openedx-authz#380](https://github.com/openedx/openedx-authz/issues/380)](openedx/openedx-authz#380).This produces exactly the same shape as an existing org-wide grant, so it is picked up by the existing
OrgRole-based legacy checks (has_staff_roles,get_user_permissions), which already check org-level and course-level access separately viaany()/or.No changes are needed to
RoleCache,OrgRole, orCourseRole. These are among the most shared and performance-sensitive code paths in this file, so keeping them untouched limits the blast radius and reduces the risk of regressions.An alternative considered was teaching
RoleCache.has_role()to understand a wildcard-org sentinel instead of fanning out one row per org. This would avoid theget_organizations()query and scale better on platforms with very large numbers of orgs.However, changing the matching semantics of one of the most widely shared methods in this file would introduce a significantly larger blast radius for review and testing, while the scalability benefit only matters at org counts for which we currently have no reports. Therefore, this approach was not pursued.
Testing Instructions
authz.enable_course_authoringflag globally.course-v1:*).has_staff_roles) for a course in any org, not just one.Added the following tests to
common/djangoapps/student/tests/test_roles.py:test_get_authz_compat_course_access_roles_for_user_platform_globtest_platform_glob_authz_role_grants_instructor_dashboard_permissionsThese mirror the existing org-wide scope test coverage.